Skip to content

quote filename shelling out to grep #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 19, 2015
Merged

quote filename shelling out to grep #7

merged 1 commit into from
Nov 19, 2015

Conversation

wfleming
Copy link
Contributor

If a path has a space in it, the grep will fail unless we quote the filename.

@@ -35,7 +35,7 @@ var printIssue = function(fileName, lineNum, matchedString){

var findFixmes = function(file){
// Prepare the grep string for execution (uses BusyBox grep)
var grepString = "grep -inHwoE " + fixmeStrings + " " + file;
var grepString = "grep -inHwoE " + fixmeStrings + " \"" + file + "\"";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I like your fix. I don't think it's better, but one other idea I had was something like:

var grepString = ["grep -inHwoE", fixmeStrings, "'" + file + "'"].join(" ");

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ABaldwinHunter You're right, we're joining enough bits now that it's not very readable. Will change in a sec.

If a path has a space in it, the grep will fail unless we quote the filename.
@wfleming
Copy link
Contributor Author

@codeclimate/review recently noticed I let this PR lie fallow. I think it's still a bug, and have updated my patch. Review please?

@fhwang
Copy link

fhwang commented Nov 19, 2015

LGTM

wfleming added a commit that referenced this pull request Nov 19, 2015
quote filename shelling out to grep
@wfleming wfleming merged commit ee589d7 into master Nov 19, 2015
@wfleming wfleming deleted the will/quote-file branch November 19, 2015 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants